swarm/metrics: Send the accounting registry to InfluxDB#1107
swarm/metrics: Send the accounting registry to InfluxDB#1107JerzyLa wants to merge 3 commits intoethersphere:masterfrom
Conversation
JerzyLa
changed the title
JerzyLa
changed the title
zelig
requested review from
acud and
nonsense
and removed request for
fjl and
zsfelfoldi
acud
left a comment
acud
left a comment
There was a problem hiding this comment.
LGTM except one small point noted
| } | ||
|
|
||
| if enableAccountingExport { | ||
| log.Info("Enabling accounting metrics export to InfluxDB") |
There was a problem hiding this comment.
this block actually sends the data to influxdb, so the log line should say something more like "exporting accounting metrics to influxdb" rather than "enabling" them. it could also be switched to log.Trace. I don't see this necessary in Info level
There was a problem hiding this comment.
I don't have strong feelings about the level of this, but it is 1 line, so sometimes it is helpful to see it, even if you are running in INFO mode.
There was a problem hiding this comment.
👍 @JerzyLa please just change the text of the log-line
| EphemeralRegistry = NewRegistry() | ||
| DefaultRegistry = NewRegistry() | ||
| EphemeralRegistry = NewRegistry() | ||
| AccountingRegistry = NewRegistry() |
There was a problem hiding this comment.
Please add a comment next to this one. Default and Ephemeral are used in geth, so we should be explicit that this registry is used within Swarm I think.
|
test fail on travis https://travis-ci.org/ethersphere/go-ethereum/jobs/479141697#L825 |
zelig
added
ready to submit to ethereum/go-ethereum
and removed
ready for review
labels
|
@JerzyLa would you submit to upstream repo please? |
|
closed by ethereum/go-ethereum#18470 |
4 participants
closes #1037
I'm still beginner contributor, please make solid review to help me make better.